326827 - Buttons don't always react to mouse clicks

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase.

To reproduce:
- Mousedown on the text in the button
- Move away with the mouse into an 'empty' area of the button
- Release the mouse button, 

Actual result:
An alert appears

Expected result:
An alert with the text 'clicked' appears

This regressed between 2006-01-25 and 2006-01-26, so likely a regression from bug 317375.

I think this is the phenomenon I see that sometimes clicking on things in the browser doesn't work.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Jesse Ruderman]

Related to bug 324396?

Comment 3

[Jesse Ruderman]

Probably not, since that regression was introduced at a different time.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Very simple fix. The button should not allow mouse events to target its children. (This is what the old GetFrameForPoint code did, I'm just copying that.)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Note that in general that may be the wrong behavior; once we fix event propagation, we want an <a> inside <button> to trigger when clicked, no?

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can you specify exactly what the right behaviour should be? If I click on an <a>, should it trigger the button as well? If I click on a <div onclick="">, should that and/or the button trigger?

Of course we'll fix this regression before worrying about that in detail...

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 211965 [details] [diff] [review]
fix

r+sr=dbaron.

In general I think there were a lot of hacks in GetFrameForPoint that didn't belong there, especially in XUL code.  Should we have a bug on auditing for such things after the event changes?

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #7)
> (From update of attachment 211965 [details] [diff] [review] [edit])
> r+sr=dbaron.
> 
> In general I think there were a lot of hacks in GetFrameForPoint that didn't
> belong there, especially in XUL code.  Should we have a bug on auditing for
> such things after the event changes?

Sure. It's really easy to audit them now, each hack was turned into an nsDisplayItem subclass :-). (Except for 'allowevents', which moved into nsDisplayList::HitTest.)

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Okay, I exaggerate slightly. You also want to look for IsForEventDelivery to see where different code paths are taken for event handling.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using build 2006-02-21-09 of SeaMonkey trunk on Windows XP on testcase: https://bugzilla.mozilla.org/attachment.cgi?id=211500&action=view

Comment 12

[Martijn Wargers (dead)]

Backing this fix out fixes bug 183113.
I guess this really would be better fixed by a fix for bug 324396.